Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use craft-application's remote-build #5239

Merged
merged 9 commits into from
Feb 14, 2025

Conversation

bepri
Copy link
Contributor

@bepri bepri commented Feb 6, 2025

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run tox run -m lint?
  • Have you successfully run tox run -e test-py310? (supported versions: py39, py310, py311, py312)

CRAFT-4044.

@bepri bepri self-assigned this Feb 6, 2025
@bepri bepri force-pushed the work/refactor-remote-build/CRAFT-4044 branch from affe7a0 to 4af108f Compare February 6, 2025 18:43
@mr-cal
Copy link
Collaborator

mr-cal commented Feb 7, 2025

I triggered a run for the remote build spread tests: https://github.com/canonical/snapcraft/actions/runs/13205639741

Copy link
Collaborator

@mr-cal mr-cal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this up for early testing. Overall, everything is working well. I found a few things that need to be addressed in craft-application:

  1. --project is failing because this needs to be called before this.

  2. remote-build: add "Pending" as a possible status output #4956 should be upstreamed. It can be out-of-scope of this task, but we should land it prior to this refactor making its way into a Snapcraft release.

  3. Using the craft-application args makes the help messages look less cohesive. Some begin with a capital letter, others don't:

Options:
                          -h, --help:  Show this help message and exit
                       -v, --verbose:  Show debug information and be
                                       more verbose
                         -q, --quiet:  Only show warnings and errors,
                                       not progress
                         --verbosity:  Set the verbosity level to
                                       'quiet', 'brief', 'verbose',
                                       'debug' or 'trace'
                       -V, --version:  Show the application version and
                                       exit
                           --recover:  recover an interrupted build
    --launchpad-accept-public-upload:  acknowledge that uploaded code
                                       will be publicly available.
                 --launchpad-timeout:  Time in seconds to wait for
                                       launchpad to build.
                           --project:  upload to the specified Launchpad
                                       project
                         --build-for:  Comma-separated list of
                                       architectures to build for

snapcraft/commands/remote.py Show resolved Hide resolved
@bepri bepri changed the title chore: use upstreamed parts of craft-application refactor: use craft-application's implementation of remote-build Feb 12, 2025
@bepri bepri changed the title refactor: use craft-application's implementation of remote-build refactor: use craft-application's remote-build Feb 12, 2025
@bepri bepri marked this pull request as ready for review February 12, 2025 21:00
tests/unit/test_application.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@mr-cal mr-cal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Still running a few tests locally but so far so good.

I also kicked off the remote-build spread tests. Let's make sure they pass before merging: https://github.com/canonical/snapcraft/actions/runs/13317525101

docs/explanation/remote-build.rst Outdated Show resolved Hide resolved
docs/reference/changelog.rst Outdated Show resolved Hide resolved
snapcraft/commands/remote.py Show resolved Hide resolved
tests/unit/commands/test_remote.py Show resolved Hide resolved
docs/explanation/remote-build.rst Outdated Show resolved Hide resolved
docs/explanation/remote-build.rst Outdated Show resolved Hide resolved
docs/explanation/remote-build.rst Outdated Show resolved Hide resolved
docs/explanation/remote-build.rst Outdated Show resolved Hide resolved
@bepri bepri requested review from medubelko and lengau and removed request for dariuszd21 February 14, 2025 16:34
docs/explanation/remote-build.rst Show resolved Hide resolved
docs/explanation/remote-build.rst Show resolved Hide resolved
@bepri bepri force-pushed the work/refactor-remote-build/CRAFT-4044 branch from 32fee25 to f4df68e Compare February 14, 2025 18:50
@bepri
Copy link
Contributor Author

bepri commented Feb 14, 2025

Rebased to fix CLA issues

@mr-cal
Copy link
Collaborator

mr-cal commented Feb 14, 2025

Merging without waiting for the core20 spread tests to complete. This PR doesn't touch core20 and I'm running the tests in the next PR, #5259.

@mr-cal mr-cal merged commit f31d994 into main Feb 14, 2025
14 of 16 checks passed
@mr-cal mr-cal deleted the work/refactor-remote-build/CRAFT-4044 branch February 14, 2025 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update craft-application remote-build: refactor onto craft-application's command
4 participants